🤖 cli-water-levels#300
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
|
jacob-a-brown
left a comment
jacob-a-brown
left a comment
There was a problem hiding this comment.
I think that this looks good for the current implementation of the water levels upload. Some changes will be coming, like making a new field for data_quality and mapping level_status to the groundwater_level_reason field, but when these updates occur these tests will be updated.
| elif table.name == "sample": | ||
| continue |
There was a problem hiding this comment.
if this was implemented because of a foreign key/non-nullable constraint from sample to field_activity that is going to be addressed in the DB models. @ksmuczynski and I talked about how we need to add cascade="DELETE" to the foreign key definition. I'm going to open a PR with this.
So, if that is the case, I think that handling samples separately from other tables can be removed from this PR.
jacob-a-brown
dismissed
their stale review
After further review and discussion with Kelsey about fixing the sample model I think that there may need to be some updates to this PR
|
I am merging this PR. I don't want PRs to remain unresolved for this long. The review comments are peripheral to the main point of this PR so it seems reasonable to merge into staging so that we are ready with cli functionality once the data and logic layers are complete |
3 participants
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
water-level-csv.featureNotes
Any special considerations, workarounds, or follow-up work to note?